From cef86134134455be201e900e4e4f9fca2e776cad Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Fri, 8 Nov 2019 12:49:43 +0100
Subject: [PATCH] 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>
(cherry picked from commit f82e3f86775b7310369aca1ee2b00222615333d2)
---
 .../netvirt/neutronvpn/NeutronvpnUtils.java   | 47 +++++++++++--------
 1 file changed, 27 insertions(+), 20 deletions(-)

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 f22f6a65bc..21c1df342b 100644
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
@@ -295,12 +295,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()))) {
@@ -320,6 +316,26 @@ 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) {
         Optional<VpnMap> optionalVpnMap = read(LogicalDatastoreType.CONFIGURATION, vpnMapIdentifier(vpnId));
@@ -400,33 +416,24 @@ public class NeutronvpnUtils {
         return NEUTRON_ROUTERS_IID.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 = NEUTRON_NETWORKS_IID.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;
         }
         LOG.debug("getNeutronPort for {}", portId.getValue());
         InstanceIdentifier<Port> inst = NEUTRON_PORTS_IID.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();
     }
 
     public PortIdToSubport getPortIdToSubport(Uuid portId) {
-- 
2.36.6