Clean up QosNeutronUtils

* Avoid returning null for collections and propagate the simplified
  null-handling.
* Use computeIfAbsent and putIfAbsent to simplify map-related code.
* Drop some local variables which are immediately returned to simplify
  declarations.
* Drop some unnecessary parentheses.

Change-Id: I701a032c1cdf1b7fc21d219476ceddc7cdc73ca5
Signed-off-by: Stephen Kitt <skitt@redhat.com>

diff --git a/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosAlertManager.java b/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosAlertManager.java
index fa097cbce09629fcd62deeb7b99b165f8d28a945..7ef30c9dfdb347df732b39e9497a6d97dd8e5f1c 100644 (file)
--- a/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosAlertManager.java
+++ b/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosAlertManager.java
@@ -236,19 +236,13 @@ public final class QosAlertManager implements Runnable {
 
         List<Uuid> subnetIds = qosNeutronUtils.getSubnetIdsFromNetworkId(network.getUuid());
 
-        if (subnetIds != null) {
-            for (Uuid subnetId : subnetIds) {
-                List<Uuid> portIds = qosNeutronUtils.getPortIdsFromSubnetId(subnetId);
-                if (portIds != null) {
-                    for (Uuid portId : portIds) {
-                        Port port = neutronVpnManager.getNeutronPort(portId);
-                        if (port != null) {
-                            if (!qosNeutronUtils.portHasQosPolicy(port)) {
-                                LOG.trace("Adding network {} port {} in cache", network.getUuid(), port.getUuid());
-                                addToQosAlertCache(port);
-                            }
-                        }
-                    }
+        for (Uuid subnetId : subnetIds) {
+            List<Uuid> portIds = qosNeutronUtils.getPortIdsFromSubnetId(subnetId);
+            for (Uuid portId : portIds) {
+                Port port = neutronVpnManager.getNeutronPort(portId);
+                if (port != null && !qosNeutronUtils.portHasQosPolicy(port)) {
+                    LOG.trace("Adding network {} port {} in cache", network.getUuid(), port.getUuid());
+                    addToQosAlertCache(port);
                 }
             }
         }
@@ -311,19 +305,13 @@ public final class QosAlertManager implements Runnable {
 
         List<Uuid> subnetIds = qosNeutronUtils.getSubnetIdsFromNetworkId(network.getUuid());
 
-        if (subnetIds != null) {
-            for (Uuid subnetId : subnetIds) {
-                List<Uuid> portIds = qosNeutronUtils.getPortIdsFromSubnetId(subnetId);
-                if (portIds != null) {
-                    for (Uuid portId : portIds) {
-                        Port port = neutronVpnManager.getNeutronPort(portId);
-                        if (port != null) {
-                            if (!qosNeutronUtils.portHasQosPolicy(port)) {
-                                LOG.trace("Removing network {} port {} from cache", network.getUuid(), port.getUuid());
-                                removeFromQosAlertCache(port);
-                            }
-                        }
-                    }
+        for (Uuid subnetId : subnetIds) {
+            List<Uuid> portIds = qosNeutronUtils.getPortIdsFromSubnetId(subnetId);
+            for (Uuid portId : portIds) {
+                Port port = neutronVpnManager.getNeutronPort(portId);
+                if (port != null && !qosNeutronUtils.portHasQosPolicy(port)) {
+                    LOG.trace("Removing network {} port {} from cache", network.getUuid(), port.getUuid());
+                    removeFromQosAlertCache(port);
                 }
             }
         }

diff --git a/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java b/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
index f2bf6f1897be3d1ea931fc20f2d65f36eb948152..1273ebeca599899ea00347e11c8c0e640b950ef0 100644 (file)
--- a/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
+++ b/vpnservice/qosservice/impl/src/main/java/org/opendaylight/netvirt/qosservice/QosNeutronUtils.java
@@ -19,6 +19,8 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -142,15 +144,7 @@ public class QosNeutronUtils {
     }
 
     public void addToQosPortsCache(Uuid qosUuid, Port port) {
-        if (qosPortsMap.containsKey(qosUuid)) {
-            if (!qosPortsMap.get(qosUuid).containsKey(port.getUuid())) {
-                qosPortsMap.get(qosUuid).put(port.getUuid(), port);
-            }
-        } else {
-            ConcurrentHashMap<Uuid, Port> portMap = new ConcurrentHashMap<>();
-            portMap.put(port.getUuid(), port);
-            qosPortsMap.put(qosUuid, portMap);
-        }
+        qosPortsMap.computeIfAbsent(qosUuid, key -> new ConcurrentHashMap<>()).putIfAbsent(port.getUuid(), port);
     }
 
     public void removeFromQosPortsCache(Uuid qosUuid, Port port) {
@@ -160,16 +154,8 @@ public class QosNeutronUtils {
     }
 
     public void addToQosNetworksCache(Uuid qosUuid, Network network) {
-        if (qosNetworksMap.containsKey(qosUuid)) {
-            if (!qosNetworksMap.get(qosUuid).containsKey(network.getUuid())) {
-                qosNetworksMap.get(qosUuid).put(network.getUuid(), network);
-            }
-        } else {
-
-            ConcurrentHashMap<Uuid, Network> networkMap = new ConcurrentHashMap<>();
-            networkMap.put(network.getUuid(), network);
-            qosNetworksMap.put(qosUuid, networkMap);
-        }
+        qosNetworksMap.computeIfAbsent(qosUuid, key -> new ConcurrentHashMap<>()).putIfAbsent(network.getUuid(),
+                network);
     }
 
     public void removeFromQosNetworksCache(Uuid qosUuid, Network network) {
@@ -178,26 +164,29 @@ public class QosNeutronUtils {
         }
     }
 
+    @Nonnull
     public Collection<Network> getQosNetworks(Uuid qosUuid) {
         final ConcurrentMap<Uuid, Network> networkMap = qosNetworksMap.get(qosUuid);
         return networkMap != null ? networkMap.values() : Collections.emptyList();
     }
 
+    @Nonnull
     public List<Uuid> getSubnetIdsFromNetworkId(Uuid networkId) {
         InstanceIdentifier<NetworkMap> networkMapId = InstanceIdentifier.builder(NetworkMaps.class)
                 .child(NetworkMap.class, new NetworkMapKey(networkId)).build();
         Optional<NetworkMap> optionalNetworkMap = MDSALUtil.read(LogicalDatastoreType.CONFIGURATION,
                 networkMapId, dataBroker);
-        return optionalNetworkMap.isPresent() ? optionalNetworkMap.get().getSubnetIdList() : null;
+        return optionalNetworkMap.isPresent() ? optionalNetworkMap.get().getSubnetIdList() : Collections.emptyList();
     }
 
+    @Nonnull
     protected List<Uuid> getPortIdsFromSubnetId(Uuid subnetId) {
         InstanceIdentifier<Subnetmap> subnetMapId = InstanceIdentifier
                 .builder(Subnetmaps.class)
                 .child(Subnetmap.class, new SubnetmapKey(subnetId)).build();
         Optional<Subnetmap> optionalSubnetmap = MDSALUtil.read(LogicalDatastoreType.CONFIGURATION,
                 subnetMapId,dataBroker);
-        return optionalSubnetmap.isPresent() ? optionalSubnetmap.get().getPortList() : null;
+        return optionalSubnetmap.isPresent() ? optionalSubnetmap.get().getPortList() : Collections.emptyList();
     }
 
     public void handleNeutronPortQosAdd(Port port, Uuid qosUuid) {
@@ -339,31 +328,26 @@ public class QosNeutronUtils {
             return;
         }
         List<Uuid> subnetIds = getSubnetIdsFromNetworkId(network.getUuid());
-        if (subnetIds != null) {
-            for (Uuid subnetId : subnetIds) {
-                List<Uuid> portIds = getPortIdsFromSubnetId(subnetId);
-                if (portIds != null) {
-                    for (Uuid portId : portIds) {
-                        Port port = neutronVpnManager.getNeutronPort(portId);
-                        if (port != null && (port.getAugmentation(QosPortExtension.class) == null
-                                || port.getAugmentation(QosPortExtension.class).getQosPolicyId() == null)) {
-                            jobCoordinator.enqueueJob("QosPort-" + portId.getValue(), () -> {
-                                WriteTransaction wrtConfigTxn = dataBroker.newWriteOnlyTransaction();
-                                List<ListenableFuture<Void>> futures = new ArrayList<>();
-                                if (qosPolicy != null && qosPolicy.getBandwidthLimitRules() != null
-                                        && !qosPolicy.getBandwidthLimitRules().isEmpty()) {
-                                    setPortBandwidthLimits(port, qosPolicy.getBandwidthLimitRules().get(0),
-                                            wrtConfigTxn);
-                                }
-                                if (qosPolicy != null && qosPolicy.getDscpmarkingRules() != null
-                                        && !qosPolicy.getDscpmarkingRules().isEmpty()) {
-                                    setPortDscpMarking(port, qosPolicy.getDscpmarkingRules().get(0));
-                                }
-                                futures.add(wrtConfigTxn.submit());
-                                return futures;
-                            });
+        for (Uuid subnetId : subnetIds) {
+            List<Uuid> portIds = getPortIdsFromSubnetId(subnetId);
+            for (Uuid portId : portIds) {
+                Port port = neutronVpnManager.getNeutronPort(portId);
+                if (port != null && (port.getAugmentation(QosPortExtension.class) == null
+                        || port.getAugmentation(QosPortExtension.class).getQosPolicyId() == null)) {
+                    jobCoordinator.enqueueJob("QosPort-" + portId.getValue(), () -> {
+                        WriteTransaction wrtConfigTxn = dataBroker.newWriteOnlyTransaction();
+                        List<ListenableFuture<Void>> futures = new ArrayList<>();
+                        if (qosPolicy.getBandwidthLimitRules() != null
+                                && !qosPolicy.getBandwidthLimitRules().isEmpty()) {
+                            setPortBandwidthLimits(port, qosPolicy.getBandwidthLimitRules().get(0),
+                                    wrtConfigTxn);
+                        }
+                        if (qosPolicy.getDscpmarkingRules() != null && !qosPolicy.getDscpmarkingRules().isEmpty()) {
+                            setPortDscpMarking(port, qosPolicy.getDscpmarkingRules().get(0));
                         }
-                    }
+                        futures.add(wrtConfigTxn.submit());
+                        return futures;
+                    });
                 }
             }
         }
@@ -374,33 +358,29 @@ public class QosNeutronUtils {
         QosPolicy qosPolicy = qosPolicyMap.get(qosUuid);
 
         List<Uuid> subnetIds = getSubnetIdsFromNetworkId(network.getUuid());
-        if (subnetIds != null) {
-            for (Uuid subnetId : subnetIds) {
-                List<Uuid> portIds = getPortIdsFromSubnetId(subnetId);
-                if (portIds != null) {
-                    for (Uuid portId : portIds) {
-                        Port port = neutronVpnManager.getNeutronPort(portId);
-                        if (port != null && (port.getAugmentation(QosPortExtension.class) == null
-                                || port.getAugmentation(QosPortExtension.class).getQosPolicyId() == null)) {
-                            jobCoordinator.enqueueJob("QosPort-" + portId.getValue(), () -> {
-                                WriteTransaction wrtConfigTxn = dataBroker.newWriteOnlyTransaction();
-                                List<ListenableFuture<Void>> futures = new ArrayList<>();
-                                if (qosPolicy != null && qosPolicy.getBandwidthLimitRules() != null
-                                        && !qosPolicy.getBandwidthLimitRules().isEmpty()) {
-                                    BandwidthLimitRulesBuilder bwLimitBuilder = new BandwidthLimitRulesBuilder();
-                                    setPortBandwidthLimits(port, bwLimitBuilder
-                                            .setMaxBurstKbps(BigInteger.ZERO)
-                                            .setMaxKbps(BigInteger.ZERO).build(), null);
-                                }
-                                if (qosPolicy != null && qosPolicy.getDscpmarkingRules() != null
-                                        && !qosPolicy.getDscpmarkingRules().isEmpty()) {
-                                    unsetPortDscpMark(port);
-                                }
-                                futures.add(wrtConfigTxn.submit());
-                                return futures;
-                            });
+        for (Uuid subnetId : subnetIds) {
+            List<Uuid> portIds = getPortIdsFromSubnetId(subnetId);
+            for (Uuid portId : portIds) {
+                Port port = neutronVpnManager.getNeutronPort(portId);
+                if (port != null && (port.getAugmentation(QosPortExtension.class) == null
+                        || port.getAugmentation(QosPortExtension.class).getQosPolicyId() == null)) {
+                    jobCoordinator.enqueueJob("QosPort-" + portId.getValue(), () -> {
+                        WriteTransaction wrtConfigTxn = dataBroker.newWriteOnlyTransaction();
+                        List<ListenableFuture<Void>> futures = new ArrayList<>();
+                        if (qosPolicy != null && qosPolicy.getBandwidthLimitRules() != null
+                                && !qosPolicy.getBandwidthLimitRules().isEmpty()) {
+                            BandwidthLimitRulesBuilder bwLimitBuilder = new BandwidthLimitRulesBuilder();
+                            setPortBandwidthLimits(port, bwLimitBuilder
+                                    .setMaxBurstKbps(BigInteger.ZERO)
+                                    .setMaxKbps(BigInteger.ZERO).build(), null);
                         }
-                    }
+                        if (qosPolicy != null && qosPolicy.getDscpmarkingRules() != null
+                                && !qosPolicy.getDscpmarkingRules().isEmpty()) {
+                            unsetPortDscpMark(port);
+                        }
+                        futures.add(wrtConfigTxn.submit());
+                        return futures;
+                    });
                 }
             }
         }
@@ -591,6 +571,7 @@ public class QosNeutronUtils {
         return nodeId;
     }
 
+    @Nullable
     private BridgeEntry getBridgeEntryFromConfigDS(BigInteger dpnId) {
         BridgeEntryKey bridgeEntryKey = new BridgeEntryKey(dpnId);
         InstanceIdentifier<BridgeEntry> bridgeEntryInstanceIdentifier = getBridgeEntryIdentifier(bridgeEntryKey);
@@ -598,24 +579,17 @@ public class QosNeutronUtils {
         return getBridgeEntryFromConfigDS(bridgeEntryInstanceIdentifier);
     }
 
+    @Nullable
     private BridgeEntry getBridgeEntryFromConfigDS(InstanceIdentifier<BridgeEntry> bridgeEntryInstanceIdentifier) {
-        Optional<BridgeEntry> bridgeEntryOptional = MDSALUtil.read(LogicalDatastoreType.CONFIGURATION,
-                bridgeEntryInstanceIdentifier, dataBroker);
-        if (!bridgeEntryOptional.isPresent()) {
-            return null;
-        }
-        return bridgeEntryOptional.get();
+        return MDSALUtil.read(LogicalDatastoreType.CONFIGURATION, bridgeEntryInstanceIdentifier, dataBroker).orNull();
     }
 
+    @Nullable
     private BridgeRefEntry getBridgeRefEntryFromOperDS(InstanceIdentifier<BridgeRefEntry> dpnBridgeEntryIid) {
-        Optional<BridgeRefEntry> bridgeRefEntryOptional = MDSALUtil.read(LogicalDatastoreType.OPERATIONAL,
-                dpnBridgeEntryIid, dataBroker);
-        if (!bridgeRefEntryOptional.isPresent()) {
-            return null;
-        }
-        return bridgeRefEntryOptional.get();
+        return MDSALUtil.read(LogicalDatastoreType.OPERATIONAL, dpnBridgeEntryIid, dataBroker).orNull();
     }
 
+    @Nullable
     private OvsdbBridgeRef getBridgeRefEntryFromOperDS(BigInteger dpId) {
         BridgeRefEntryKey bridgeRefEntryKey = new BridgeRefEntryKey(dpId);
         InstanceIdentifier<BridgeRefEntry> bridgeRefEntryIid = getBridgeRefEntryIdentifier(bridgeRefEntryKey);
@@ -632,17 +606,14 @@ public class QosNeutronUtils {
         return bridgeRefEntry.getBridgeReference();
     }
 
+    @Nonnull
     private static InstanceIdentifier<BridgeRefEntry> getBridgeRefEntryIdentifier(BridgeRefEntryKey bridgeRefEntryKey) {
-        InstanceIdentifier.InstanceIdentifierBuilder<BridgeRefEntry> bridgeRefEntryInstanceIdentifierBuilder =
-                InstanceIdentifier.builder(BridgeRefInfo.class)
-                        .child(BridgeRefEntry.class, bridgeRefEntryKey);
-        return bridgeRefEntryInstanceIdentifierBuilder.build();
+        return InstanceIdentifier.builder(BridgeRefInfo.class).child(BridgeRefEntry.class, bridgeRefEntryKey).build();
     }
 
+    @Nonnull
     private static InstanceIdentifier<BridgeEntry> getBridgeEntryIdentifier(BridgeEntryKey bridgeEntryKey) {
-        InstanceIdentifier.InstanceIdentifierBuilder<BridgeEntry> bridgeEntryIdBuilder =
-                InstanceIdentifier.builder(BridgeInterfaceInfo.class).child(BridgeEntry.class, bridgeEntryKey);
-        return bridgeEntryIdBuilder.build();
+        return InstanceIdentifier.builder(BridgeInterfaceInfo.class).child(BridgeEntry.class, bridgeEntryKey).build();
     }
 
     public void removeStaleFlowEntry(Interface intrf) {
@@ -697,32 +668,26 @@ public class QosNeutronUtils {
                 new FlowId(getQosFlowId(NwConstants.QOS_DSCP_TABLE, dpnId, ifIndex)));
     }
 
+    @Nullable
     public org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
             .ietf.interfaces.rev140508.interfaces.state.Interface getInterfaceStateFromOperDS(
             String interfaceName) {
-        InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
-                .ietf.interfaces.rev140508.interfaces.state.Interface> ifStateId =
-                createInterfaceStateInstanceIdentifier(interfaceName);
-        Optional<org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
-                .ietf.interfaces.rev140508.interfaces.state.Interface> ifStateOptional = MDSALUtil
-                .read(dataBroker, LogicalDatastoreType.OPERATIONAL, ifStateId);
-        if (ifStateOptional.isPresent()) {
-            return ifStateOptional.get();
-        }
-        return null;
+        return MDSALUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL,
+                createInterfaceStateInstanceIdentifier(interfaceName)).orNull();
     }
 
+    @Nonnull
     public static InstanceIdentifier<org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
             .ietf.interfaces.rev140508.interfaces.state.Interface> createInterfaceStateInstanceIdentifier(
             String interfaceName) {
-        InstanceIdentifier.InstanceIdentifierBuilder<Interface> idBuilder = InstanceIdentifier
+        return InstanceIdentifier
                 .builder(InterfacesState.class)
                 .child(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
                                 .ietf.interfaces.rev140508.interfaces.state.Interface.class,
                         new org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang
                                 .ietf.interfaces.rev140508.interfaces.state.InterfaceKey(
-                                interfaceName));
-        return idBuilder.build();
+                                interfaceName))
+                .build();
     }
 
     public void bindservice(String ifName) {
@@ -760,8 +725,9 @@ public class QosNeutronUtils {
                 .addAugmentation(StypeOpenflow.class, augBuilder.build()).build();
     }
 
+    @Nonnull
     public static String getQosFlowId(short tableId, BigInteger dpId, int lportTag) {
-        return new StringBuffer().append(tableId).append(dpId).append(lportTag).toString();
+        return String.valueOf(tableId) + dpId + lportTag;
     }
 
     public String getPortNumberForInterface(String ifName) {
@@ -780,17 +746,10 @@ public class QosNeutronUtils {
     }
 
     public boolean portHasQosPolicy(Port port) {
-        Uuid qosUuid = null;
-        boolean isQosPolicy = false;
-
         LOG.trace("checking qos policy for port: {}", port.getUuid());
 
-        if (port.getAugmentation(QosPortExtension.class) != null) {
-            qosUuid = port.getAugmentation(QosPortExtension.class).getQosPolicyId();
-        }
-        if (qosUuid != null) {
-            isQosPolicy = true;
-        }
+        boolean isQosPolicy = port.getAugmentation(QosPortExtension.class) != null
+                && port.getAugmentation(QosPortExtension.class).getQosPolicyId() != null;
 
         LOG.trace("portHasQosPolicy for  port: {} return value {}", port.getUuid(), isQosPolicy);
         return isQosPolicy;
@@ -845,6 +804,7 @@ public class QosNeutronUtils {
         return bwLimitRule;
     }
 
+    @Nullable
     public QosPolicy getQosPolicy(Port port) {
         Uuid qosUuid = null;
         QosPolicy qosPolicy = null;