BE: NMS-16536, Provisioning fails when category has been deleted (#7463)

* NMS-16536, initial commit with fixes

* NMS-16536, additional logs added to trace the issue, condition updated to avoid frequent reloading

* NMS-16536, added unit test, removed extra logging, added for bug tracing.

* NMS-16536, added unit test, removed extra logging, added for bug tracing.

* NMS-16536, optimized imports, reverted file permission

* NMS-16536, reverted files permissions

* NMS-16536, added IT for review

* NMS-16536, updated test case

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<String, OnmsCategory> categoryCache) {
+        m_categoryCache.set(categoryCache);
+    }
+
     /**
      * <p>getForeignSourceRepository</p>
      *
@@ -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);

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<String> checkSetsOfCategories(Set<String> reqCats, Set<String> 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<OnmsCategory> 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;
+    }
 }

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 {

opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories-oneDeleted.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<model-import last-import="2012-05-03T03:51:43.022-05:00" foreign-source="empty" date-stamp="2012-05-03T03:51:42.441-05:00" xmlns="http://xmlns.opennms.org/xsd/config/model-import">
+    <node node-label="test" foreign-id="1">
+        <interface snmp-primary="P" status="1" ip-addr="172.16.1.1" descr="eth0">
+            <monitored-service service-name="ICMP"/>
+        </interface>
+        <category name="blarbs" />
+        <category name="foops" />
+    </node>
+</model-import>

opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<model-import last-import="2012-05-03T03:51:43.022-05:00" foreign-source="empty" date-stamp="2012-05-03T03:51:42.441-05:00" xmlns="http://xmlns.opennms.org/xsd/config/model-import">
+    <node node-label="test" foreign-id="1">
+        <interface snmp-primary="P" status="1" ip-addr="172.16.1.1" descr="eth0">
+            <monitored-service service-name="ICMP"/>
+        </interface>
+        <category name="blarbs" />
+        <category name="foops" />
+        <category name="NMS-16536" />
+    </node>
+</model-import>