Bug 1348 - TopologyCommitHandler NullPointerException on edgeUpdate

CHANGED

* added null check for non-existent edge properties
* added test to test if issue is fixed

Change-Id: I0e2b2f9502be47ac87be5e4213068bcb45295366
Signed-off-by: Jan Hajnar <jhajnar@cisco.com>

diff --git a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java
index b0e87c48f3e0812fd2e33c7789e5e916479f6465..659ee7dd81ca83d91c013ceddb7017edca9a8b1b 100644 (file)
--- a/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java
+++ b/opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java
@@ -8,25 +8,6 @@
 
 package org.opendaylight.controller.topologymanager.internal;
 
 
 package org.opendaylight.controller.topologymanager.internal;
 

-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.util.ArrayList;
-import java.util.Dictionary;
-import java.util.EnumSet;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.CopyOnWriteArraySet;
-import java.util.concurrent.LinkedBlockingQueue;
-

 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.felix.dm.Component;
 import org.eclipse.osgi.framework.console.CommandInterpreter;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.felix.dm.Component;
 import org.eclipse.osgi.framework.console.CommandInterpreter;


@@ -64,6 +45,25 @@ import org.osgi.framework.FrameworkUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 

+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.util.ArrayList;
+import java.util.Dictionary;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.LinkedBlockingQueue;
+

 /**
  * The class describes TopologyManager which is the central repository of the
  * network topology. It provides service for applications to interact with
 /**
  * The class describes TopologyManager which is the central repository of the
  * network topology. It provides service for applications to interact with


@@ -654,12 +654,14 @@ public class TopologyManagerImpl implements
             // all except the creation time stamp because that should
             // be set only when the edge is created
             TimeStamp timeStamp = null;
             // all except the creation time stamp because that should
             // be set only when the edge is created
             TimeStamp timeStamp = null;

-            for (Property prop : oldProps) {
-                if (prop instanceof TimeStamp) {
-                    TimeStamp tsProp = (TimeStamp) prop;
-                    if (tsProp.getTimeStampName().equals("creation")) {
-                        timeStamp = tsProp;
-                        break;
+            if (oldProps != null) {
+                for (Property prop : oldProps) {
+                    if (prop instanceof TimeStamp) {
+                        TimeStamp tsProp = (TimeStamp) prop;
+                        if (tsProp.getTimeStampName().equals("creation")) {
+                            timeStamp = tsProp;
+                            break;
+                        }

                     }
                 }
             }
                     }
                 }
             }


@@ -679,7 +681,9 @@ public class TopologyManagerImpl implements
                 if (prop instanceof TimeStamp) {
                     TimeStamp t = (TimeStamp) prop;
                     if (t.getTimeStampName().equals("creation")) {
                 if (prop instanceof TimeStamp) {
                     TimeStamp t = (TimeStamp) prop;
                     if (t.getTimeStampName().equals("creation")) {

-                        i.remove();
+                        if (timeStamp != null) {
+                            i.remove();
+                        }

                         break;
                     }
                 }
                         break;
                     }
                 }

diff --git a/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java b/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java
index fa01fa6a6025f1dc4da35e0bda80b43f77a0388a..d1338bf6953909aff8ff1c4bea274001f9135e5c 100644 (file)
--- a/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java
+++ b/opendaylight/topologymanager/implementation/src/test/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImplTest.java
@@ -8,21 +8,11 @@
 
 package org.opendaylight.controller.topologymanager.internal;
 
 
 package org.opendaylight.controller.topologymanager.internal;
 

-import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ConcurrentMap;
-

 import org.junit.Assert;
 import org.junit.Test;
 import org.opendaylight.controller.sal.core.Bandwidth;
 import org.opendaylight.controller.sal.core.ConstructionException;
 import org.junit.Assert;
 import org.junit.Test;
 import org.opendaylight.controller.sal.core.Bandwidth;
 import org.opendaylight.controller.sal.core.ConstructionException;

+import org.opendaylight.controller.sal.core.Description;

 import org.opendaylight.controller.sal.core.Edge;
 import org.opendaylight.controller.sal.core.Host;
 import org.opendaylight.controller.sal.core.Latency;
 import org.opendaylight.controller.sal.core.Edge;
 import org.opendaylight.controller.sal.core.Host;
 import org.opendaylight.controller.sal.core.Latency;


@@ -32,6 +22,7 @@ import org.opendaylight.controller.sal.core.NodeConnector;
 import org.opendaylight.controller.sal.core.NodeConnector.NodeConnectorIDType;
 import org.opendaylight.controller.sal.core.Property;
 import org.opendaylight.controller.sal.core.State;
 import org.opendaylight.controller.sal.core.NodeConnector.NodeConnectorIDType;
 import org.opendaylight.controller.sal.core.Property;
 import org.opendaylight.controller.sal.core.State;

+import org.opendaylight.controller.sal.core.TimeStamp;

 import org.opendaylight.controller.sal.core.UpdateType;
 import org.opendaylight.controller.sal.packet.address.EthernetAddress;
 import org.opendaylight.controller.sal.topology.TopoEdgeUpdate;
 import org.opendaylight.controller.sal.core.UpdateType;
 import org.opendaylight.controller.sal.packet.address.EthernetAddress;
 import org.opendaylight.controller.sal.topology.TopoEdgeUpdate;


@@ -47,6 +38,17 @@ import org.opendaylight.controller.switchmanager.Switch;
 import org.opendaylight.controller.switchmanager.SwitchConfig;
 import org.opendaylight.controller.topologymanager.TopologyUserLinkConfig;
 
 import org.opendaylight.controller.switchmanager.SwitchConfig;
 import org.opendaylight.controller.topologymanager.TopologyUserLinkConfig;
 

+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentMap;
+

 public class TopologyManagerImplTest {
     /**
      * Mockup of switch manager that only maintains existence of node
 public class TopologyManagerImplTest {
     /**
      * Mockup of switch manager that only maintains existence of node


@@ -733,4 +735,35 @@ public class TopologyManagerImplTest {
 
         Assert.assertTrue(nodeNCmap.isEmpty());
     }
 
         Assert.assertTrue(nodeNCmap.isEmpty());
     }

+
+    @Test
+    public void bug1348FixTest() throws ConstructionException {
+        TopologyManagerImpl topoManagerImpl = new TopologyManagerImpl();
+        TestSwitchManager swMgr = new TestSwitchManager();
+        topoManagerImpl.setSwitchManager(swMgr);
+        topoManagerImpl.nonClusterObjectCreate();
+
+        NodeConnector headnc1 = NodeConnectorCreator.createOFNodeConnector(
+                (short) 1, NodeCreator.createOFNode(1000L));
+        NodeConnector tailnc1 = NodeConnectorCreator.createOFNodeConnector(
+                (short) 2, NodeCreator.createOFNode(2000L));
+        Edge edge = new Edge(headnc1, tailnc1);
+        List<TopoEdgeUpdate> updatedEdges = new ArrayList<>();
+        Set<Property> edgeProps = new HashSet<>();
+        edgeProps.add(new TimeStamp(System.currentTimeMillis(), "creation"));
+        edgeProps.add(new Latency(Latency.LATENCY100ns));
+        edgeProps.add(new State(State.EDGE_UP));
+        edgeProps.add(new Bandwidth(Bandwidth.BW100Gbps));
+        edgeProps.add(new Description("Test edge"));
+        updatedEdges.add(new TopoEdgeUpdate(edge, edgeProps, UpdateType.CHANGED));
+
+        try {
+            topoManagerImpl.edgeUpdate(updatedEdges);
+        } catch (Exception e) {
+            Assert.fail("Exception was raised when trying to update edge properties: " + e.getMessage());
+        }
+
+        Assert.assertEquals(1, topoManagerImpl.getEdges().size());
+        Assert.assertNotNull(topoManagerImpl.getEdges().get(edge));
+    }

 }
 }