From 4641f9e79e3045d17cee26c733eba8aa0b9edd43 Mon Sep 17 00:00:00 2001 From: Jan Hajnar Date: Thu, 11 Sep 2014 15:13:17 +0200 Subject: [PATCH] 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 --- .../internal/TopologyManagerImpl.java | 56 ++++++++++--------- .../internal/TopologyManagerImplTest.java | 55 ++++++++++++++---- 2 files changed, 74 insertions(+), 37 deletions(-) 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 b0e87c48f3..659ee7dd81 100644 --- 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; -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; @@ -64,6 +45,25 @@ import org.osgi.framework.FrameworkUtil; 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 @@ -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; - 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")) { - i.remove(); + if (timeStamp != null) { + i.remove(); + } 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 fa01fa6a60..d1338bf695 100644 --- 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; -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.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; @@ -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.TimeStamp; 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 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 @@ -733,4 +735,35 @@ public class TopologyManagerImplTest { 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 updatedEdges = new ArrayList<>(); + Set 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)); + } } -- 2.36.6