Check network presence

In case we end up with an UUID for which we cannot find a network,
we end up with this splat:

java.lang.NullPointerException: null
	at org.opendaylight.netvirt.neutronvpn.NeutronvpnUtils.getIsExternal(NeutronvpnUtils.java:988) ~[?:?]
	at org.opendaylight.netvirt.neutronvpn.NeutronvpnUtils.getVpnForRouter(NeutronvpnUtils.java:291) ~[?:?]
	at org.opendaylight.netvirt.neutronvpn.NeutronvpnManager.createL3InternalVpn(NeutronvpnManager.java:1203) ~[?:?]
	at org.opendaylight.netvirt.neutronvpn.NeutronRouterChangeListener.add(NeutronRouterChangeListener.java:84) ~[?:?]
	at org.opendaylight.netvirt.neutronvpn.NeutronRouterChangeListener.add(NeutronRouterChangeListener.java:36) ~[?:?]
	at org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase$DataTreeChangeHandler.run(AsyncDataTreeChangeListenerBase.java:171) ~[274:org.opendaylight.genius.mdsalutil-api:0.7.1]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[?:?]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) ~[?:?]
	at java.lang.Thread.run(Thread.java:748) [?:?]

This adds appropriate annotations so that callers of getNetwork()
are flagged when they do not handle nulls. Fixes the reported offender
by moving the checking code into a utility class.

JIRA: NETVIRT-1636
Change-Id: I8872699a1bbf56a55977a9f1e8a0d97b1ff45f94
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
index 27806c64fc26ae24044fd1a649d371a44e090171..f59656cba687953db91e917c96235df7d910e256 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
@@ -285,12 +285,8 @@ public class NeutronvpnUtils {
                     continue;
                 }
                 // Skip router vpnId fetching from internet BGP-VPN
-                if (vpnMap.getNetworkIds() != null && !vpnMap.getNetworkIds().isEmpty()) {
-                    // We only need to check the first network; if it’s not an external network there’s no
-                    // need to check the rest of the VPN’s network list
-                    if (getIsExternal(getNeutronNetwork(vpnMap.getNetworkIds().iterator().next()))) {
-                        continue;
-                    }
+                if (hasExternalNetwork(vpnMap.getNetworkIds())) {
+                    continue;
                 }
                 // FIXME: NETVIRT-1503: this check can be replaced by a ReadOnlyTransaction.exists()
                 if (routerIdsList.stream().anyMatch(routerIds -> routerId.equals(routerIds.getRouterId()))) {
@@ -310,6 +306,25 @@ public class NeutronvpnUtils {
         return null;
     }
 
+    // We only need to check the first network; if it’s not an external network there’s no
+    // need to check the rest of the VPN’s network list. Note that some UUIDs may point to unknown networks, in which
+    // case we check more  and assume false.
+    private boolean hasExternalNetwork(List<Uuid> uuids) {
+        if (uuids != null) {
+            for (Uuid uuid : uuids) {
+                final Network network = getNeutronNetwork(uuid);
+                if (network != null) {
+                    if (Boolean.TRUE.equals(getIsExternal(network))) {
+                        return true;
+                    }
+                } else {
+                    LOG.debug("hasExternalNetwork: cannot find network for {}", uuid);
+                }
+            }
+        }
+        return false;
+    }
+
     @Nullable
     protected List<Uuid> getRouterIdListforVpn(Uuid vpnId) {
         InstanceIdentifier<VpnMap> vpnMapIdentifier = InstanceIdentifier.builder(VpnMaps.class).child(VpnMap.class,
@@ -390,28 +405,22 @@ public class NeutronvpnUtils {
     }
 
     public InstanceIdentifier<Router> getNeutronRouterIid(Uuid routerId) {
-        return InstanceIdentifier.create(Neutron.class).child(Routers.class).child(Router
-                .class, new RouterKey(routerId));
-
+        return InstanceIdentifier.create(Neutron.class).child(Routers.class)
+                .child(Router.class, new RouterKey(routerId));
     }
 
-    protected Network getNeutronNetwork(Uuid networkId) {
-        Network network = null;
-        network = networkMap.get(networkId);
+    protected @Nullable Network getNeutronNetwork(Uuid networkId) {
+        Network network = networkMap.get(networkId);
         if (network != null) {
             return network;
         }
         LOG.debug("getNeutronNetwork for {}", networkId.getValue());
         InstanceIdentifier<Network> inst = InstanceIdentifier.create(Neutron.class).child(Networks.class)
             .child(Network.class, new NetworkKey(networkId));
-        Optional<Network> net = read(LogicalDatastoreType.CONFIGURATION, inst);
-        if (net.isPresent()) {
-            network = net.get();
-        }
-        return network;
+        return read(LogicalDatastoreType.CONFIGURATION, inst).orNull();
     }
 
-    protected Port getNeutronPort(Uuid portId) {
+    protected @Nullable Port getNeutronPort(Uuid portId) {
         Port prt = portMap.get(portId);
         if (prt != null) {
             return prt;
@@ -419,11 +428,7 @@ public class NeutronvpnUtils {
         LOG.debug("getNeutronPort for {}", portId.getValue());
         InstanceIdentifier<Port> inst = InstanceIdentifier.create(Neutron.class).child(Ports.class).child(Port.class,
                 new PortKey(portId));
-        Optional<Port> port = read(LogicalDatastoreType.CONFIGURATION, inst);
-        if (port.isPresent()) {
-            prt = port.get();
-        }
-        return prt;
+        return read(LogicalDatastoreType.CONFIGURATION, inst).orNull();
     }
 
     /**